fix: acknowledge label-free sweep reuse / 无需扫描标签即可复用并反馈结果 - #2986
Conversation
让 /reuse-sweep-run 无需当前扫描标签即可复用符合条件的产物，并通过原评论上的赞或踩反馈验证结果。将 GitHub 基础操作和复用策略分别收敛到 infx.github 和 infx.workflows，保留旧命令行入口，并补充行为和失败路径测试。
|
Claude finished @adibarra's task in 4m 22s —— View job Review of PR #2986
Summary: The refactor into One 🟡 warning inline on Two non-blocking notes, both consistent with what the PR body already states:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| """Select and validate the same source at acknowledgment, PR sync, and merge.""" | ||
| incompatible_present = incompatible_labels.intersection(label_names(pr)) | ||
| if incompatible_present: | ||
| names = ", ".join(sorted(incompatible_present)) | ||
| raise RuntimeError( | ||
| f"PR #{pr_number} has {command} authorization but " | ||
| f"uses reuse-incompatible label(s): {names}." | ||
| ) |
There was a problem hiding this comment.
🟡 WARNING: Nothing here verifies the source run was a full sweep anymore. The removed full-sweep label check was the only guard distinguishing a full-sweep source from a sweep-enabled trim sweep, and trim sweeps also upload results_bmk.
Why it matters: An unpinned /reuse-sweep-run on a sweep-enabled PR (or any PR whose only completed run was a trim sweep) now validates, skips the PR sweep on synchronize, and at merge makes the push-to-main run reuse a partial-matrix artifact set instead of running the full main sweep. utils/changelog_gate_tests/test_merge_with_reuse.py explicitly encodes ["sweep-enabled"] -> allowed, so this is reachable through the supported merge path, not only manual comments. .github/codeowner-signoff-verify-prompt.md still states the merge standard "requires a green full sweep".
Fix: Either (a) accept this and state in the reuse docs that a trim-sweep source yields a partial main ingest, or (b) keep a source-shape check that does not depend on the current label — e.g. reject sweep-enabled in incompatible_labels for the unpinned path, or require the pinned form when the PR carries sweep-enabled:
| """Select and validate the same source at acknowledgment, PR sync, and merge.""" | |
| incompatible_present = incompatible_labels.intersection(label_names(pr)) | |
| if incompatible_present: | |
| names = ", ".join(sorted(incompatible_present)) | |
| raise RuntimeError( | |
| f"PR #{pr_number} has {command} authorization but " | |
| f"uses reuse-incompatible label(s): {names}." | |
| ) | |
| """Select and validate the same source at acknowledgment, PR sync, and merge.""" | |
| incompatible_present = incompatible_labels.intersection(label_names(pr)) | |
| if incompatible_present: | |
| names = ", ".join(sorted(incompatible_present)) | |
| raise RuntimeError( | |
| f"PR #{pr_number} has {command} authorization but " | |
| f"uses reuse-incompatible label(s): {names}." | |
| ) | |
| if pinned_run_id is None and "sweep-enabled" in label_names(pr): | |
| raise RuntimeError( | |
| f"PR #{pr_number} carries sweep-enabled (trim sweep); pin the full-sweep " | |
| f"run explicitly with `{command} <run_id>`." | |
| ) |
明确说明裁剪扫描也可被自动选择用于复用，但只会发布已有数据点；复用请求被接受不代表已满足完整扫描评审要求。
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked whether the uncaught RuntimeError from resolve_reusable_run() in the pull_request/synchronize path (reuse-sweep-gate job in infx/workflows/reuse.py) is a silent-failure risk similar to the comment-acknowledgment bug flagged inline — it isn't: an uncaught exception there fails the reuse-sweep-gate step/job itself, and the downstream setup job's if: condition requires that job to be skipped or success, so an invalid/stale reuse authorization actually blocks the whole sweep pipeline (fail-closed) rather than being swallowed.
Extended reasoning...
Two confirmed findings are already queued as inline comments: an uncaught exception risk in reuse_comment.py's acknowledge() (API calls outside the try/except that can crash before any reaction/summary is written), and a workflow-level concurrency: group in reuse-sweep-comment.yml keyed only on issue number that can cancel unrelated in-flight runs for the same PR. Given these findings exist, per the review protocol I either stay silent or add a brief defer note about something concrete I examined beyond them. I traced the ruled-out candidate about resolve_reusable_run() raising uncaught in the pull_request/synchronize main() branch (infx/workflows/reuse.py around lines 358-371) through to .github/workflows/run-sweep.yml's reuse-sweep-gate job (which runs the script directly with no error suppression) and the downstream setup job's if: gating (needs.reuse-sweep-gate.result == 'skipped' || (needs.reuse-sweep-gate.result == 'success' && ...)). This confirms the uncaught exception there is a fail-closed control-flow choice — it halts the sweep, matching the PR's stated "invalid sources fail closed" intent — rather than a silent-failure bug like the comment-path issue. This is worth a short note since it clarifies why that ruled-out candidate is not equivalent to the confirmed comment-path finding, without restating either.
| comment = github.api(repo, comment_path, token) | ||
| # A queued event must not acknowledge an edited version it did not validate. | ||
| if any(comment.get(key) != event["comment"].get(key) for key in ("body", "updated_at")): | ||
| return 0 | ||
| github.set_comment_reaction(repo, comment_id, token, None, replace=("+1", "-1")) |
There was a problem hiding this comment.
🟡 (optional) The comment re-fetch (github.api) and the initial reaction-clear (github.set_comment_reaction) run outside the try/except block, so any transient GitHub API failure there (5xx, rate limit, timeout) raises uncaught and crashes acknowledge()/main() before either a 👍/👎 reaction or a GITHUB_STEP_SUMMARY entry is ever written. This defeats the PR's core promise of always giving immediate reaction feedback: the requester instead just sees a failed Action run with no visible acknowledgment. Fix: wrap the whole body (from the initial re-fetch through the final reaction) in one try/except so any failure still yields a rejection reaction and a summary line, or explicitly document/handle the fetch step as best-effort. [also at: infx/workflows/reuse_comment.py:65 - The except handler's own re-check call github.api(repo, comment_path, token) (line 60) is not wrapped in try/except,…]
Extended reasoning...
acknowledge() calls comment = github.api(repo, comment_path, token) (line 23) then github.set_comment_reaction(..., None, replace=("+1","-1")) (line 27) before entering the try: block that starts later. github.api raises RuntimeError on any HTTPError (including a transient 500/502/403-rate-limit), and set_comment_reaction internally calls paginate/api too, both able to raise the same way. Neither call is inside the try/except that produces the -1 reaction and the Reuse rejected summary message, and main() has no top-level exception handler either — it just does return acknowledge(...) then sys.exit(main()). So on any transient error at that point, the job fails with a traceback, the comment gets no reaction at all (not even 👎), and no GITHUB_STEP_SUMMARY line is written, unlike every other rejection path in this same function which is caught and reported. This runs on every issue_comment created/edited event whose body (or prior body) contains /reuse-sweep-run,…
Verification: nit. Factually accurate: in infx/workflows/reuse_comment.py, line 23 comment = github.api(repo, comment_path, token) and line 27 github.set_comment_reaction(repo, comment_id, token, None, replace=("+1","-1")) are both outside the try: that starts at line 32. github.api raises RuntimeError on any HTTPError (infx/github.py:41-43), and set_comment_reaction with replace=("+1","-1")…
| on: | ||
| issue_comment: | ||
| types: [created, edited] | ||
|
|
||
| permissions: {} | ||
|
|
||
| concurrency: | ||
| group: reuse-comment-${{ github.event.issue.number }} | ||
| cancel-in-progress: false |
There was a problem hiding this comment.
🟡 (optional) The concurrency: block is at workflow level (keyed only by issue number, no body filter), but on: issue_comment: types:[created, edited] fires for every comment/edit on the PR, not just /reuse-sweep-run ones — the content check only lives in the job's if:. Any unrelated comment or edit on a busy PR queues a new workflow run in the same group, which GitHub Actions uses to cancel the previously queued (not-yet-started) run, silently dropping an in-flight acknowledgment for a real /reuse-sweep-run request with no reaction and no retry, defeating the PR's stated 'immediate acknowledgment' goal. …
Extended reasoning...
…Fix: scope concurrency (or an early exit) to comments that actually match the reuse command, e.g. include a hash of matched content in the group key, or move the filter to the workflow's on.issue_comment level so unrelated activity doesn't enter the same queue.
Workflow-level concurrency: group: reuse-comment-${{ number }} applies to every triggered run, since on: has no path/body filter; the job's contains(...) check is evaluated only after the run is dequeued. GitHub Actions concurrency semantics: a currently pending (queued) run in a group is canceled outright when a newer run for the same group arrives, regardless of cancel-in-progress. Sequence: comment A posts /reuse-sweep-run 123; run for A queues behind an already-running unrelated-comment run B; before B finishes, unrelated comment/edit C lands on the same PR, its run enters the queue and cancels A's pending run. A's acknowledge() never executes — no +1/-1 reaction, no rejection message, and the maintainer has no signal the request was dropped rather than accepted or rejected.
Verification: nit. Mechanism is real and matches documented GitHub Actions behavior. on: issue_comment: types:[created,edited] (lines 4-6) has no body filter, and the concurrency group is workflow-level keyed only by issue number (lines 10-12: group: reuse-comment-${{ github.event.issue.number }}, cancel-in-progress: false); the /reuse-sweep-run filter exists only in the job if: (lines 16-19),…
…isAI#2986) * fix: acknowledge label-free sweep reuse through infx 让 /reuse-sweep-run 无需当前扫描标签即可复用符合条件的产物，并通过原评论上的赞或踩反馈验证结果。将 GitHub 基础操作和复用策略分别收敛到 infx.github 和 infx.workflows，保留旧命令行入口，并补充行为和失败路径测试。 * docs: clarify partial coverage when reusing trimmed sweeps 明确说明裁剪扫描也可被自动选择用于复用，但只会发布已有数据点；复用请求被接受不代表已满足完整扫描评审要求。
1 participant
An authorized
/reuse-sweep-runrequest could have valid artifacts but fail because the PR no longer had a full-sweep label. Requests also had no immediate acknowledgment. This change removes the current-label requirement and validates requests when they are posted or edited, with 👍 for acceptance and 👎 for rejection on the original comment. Rejection details appear in the Actions summary; no additional comment is posted.已授权的
/reuse-sweep-run请求即使有可用产物，也可能因为 PR 不再带有完整扫描标签而失败，而且此前没有即时反馈。本次改动移除对当前扫描标签的要求，在评论创建或编辑时验证请求，并在原评论上用 👍 表示接受、👎 表示拒绝。拒绝原因写入 Actions 摘要，不额外发布评论。Share source selection and validation across comment acknowledgment, PR synchronization, and main-branch reuse. Invalid sources fail closed. Existing author permissions, source-run checks, and
evals-only/agentx-fastexclusions remain. Explicit failed/cancelled runs can still supply available artifacts; this does not turn them into passing sweeps.Put GitHub API, pagination, and reaction primitives in
infx.github, with reuse policy and command handling ininfx.workflows. Keep the existing script entry point compatible. The comment workflow loads trusted default-branch code and usesGITHUB_TOKEN.Preserve human reactions, clear obsolete bot status on edits, and recheck comments before acknowledgment. Reactions are informational: merge-time validation still applies. Requests without a run ID retain latest-successful-run selection; the merge helper now pins the exact run verified during preflight.
Source validation checks identity and available artifacts, not full-matrix coverage. Successful trimmed sources are eligible, including automatic selection, and publish only their recorded points on
main. Acceptance does not certify a green full sweep or replace the review requirement; verify coverage and pin the source when a full sweep is required.评论反馈、PR 同步和 main 分支复用共用产物选择与验证逻辑；来源无效时拒绝复用。保留现有作者权限、来源运行检查及
evals-only/agentx-fast排除规则。显式指定失败或取消的运行仍可复用已有产物，但不代表扫描通过。将 GitHub API、分页和 reaction 基础操作放入
infx.github，将复用策略与命令处理放入infx.workflows，并保留旧脚本入口。评论工作流加载默认分支上的可信代码，使用GITHUB_TOKEN。保留人工 reaction，编辑后清理过期的机器人状态，并在反馈前再次检查评论。reaction 仅用于提示，合并时仍会重新验证。未指定运行 ID 的请求继续选择最近一次成功运行；合并脚本则固定使用预检验证过的运行 ID。
来源验证检查身份和可用产物，不检查完整矩阵覆盖范围。成功的裁剪扫描也可复用，包括自动选择；在
main上只发布其已记录的数据点。请求被接受不代表已通过完整扫描，也不能代替评审要求；需要完整扫描时，应先确认覆盖范围，再固定源运行。Validation: the affected CI test command passed 657 tests in 8.94s on Python 3.12.13. Tests exercise real parsing/selection, label-free reuse, invalid and stale requests, API failures, reaction ownership, workflow routing, legacy/package CLI behavior, and merge-helper behavior with external calls stubbed.
actionlint, shell syntax validation, andgit diff --checkpassed. No benchmark runtime, recipe, or dependency changes.验证：受影响的 CI 测试命令在 Python 3.12.13 上通过 657 项测试，耗时 8.94 秒。测试覆盖实际解析与选择逻辑、无标签复用、无效或过期请求、API 失败、reaction 归属、工作流路由、旧脚本及包命令行入口，以及通过替身隔离外部调用的合并脚本行为。
actionlint、Shell 语法检查和git diff --check均通过。未修改基准测试运行逻辑、recipe 或依赖。Deployment note: GitHub activates the new
issue_commentworkflow after it lands on the default branch. Local validation covers its behavior with mocked GitHub responses; live reaction delivery has not been tested before merge.部署说明：新的
issue_comment工作流进入默认分支后才会由 GitHub 触发。本地已使用模拟的 GitHub 响应验证其行为，尚未在合并前验证真实 reaction 的发送。Note
Medium Risk
Changes merge-time CI behavior and reuse eligibility (including trimmed sweeps), so incorrect source selection could publish partial benchmark data on
maindespite stricter revalidation at sync and merge.Overview
Sweep artifact reuse no longer depends on keeping a full-sweep label on the PR, and maintainers get immediate feedback when they comment
/reuse-sweep-run.Reuse selection and validation move into
infx.github(REST/pagination/reactions) andinfx/workflows/reuse/reuse_comment, withutils/find_reusable_sweep_run.pykept as a thin compatibility shim.run-sweep.ymlinvokespython3 -m infx.workflows.reuseand broadensreuse-sweep-gateso authorized reuse can skip PR sweeps on sync without a primary sweep label (still excludingevals-only/agentx-fast). On sync, the gate now validates the source run before settingskip-pr-sweep.A new
reuse-sweep-comment.ymlworkflow reacts on create/edit with 👍/👎 (details in the Actions summary, no extra comment).merge_with_reuse.shdrops the full-sweep-label requirement, allows at most one primary label, and posts/reuse-sweep-run <run_id>pinned to the preflight run. Docs and tests are updated to state that acceptance checks artifacts and identity, not full-matrix coverage (trimsweep-enabledruns can be selected unless pinned).Reviewed by Cursor Bugbot for commit 5b55fc8. Bugbot is set up for automated code reviews on this repo. Configure here.